Add method-based TagLib syntax with legacy compatibility, benchmarks, and docs#15465
Add method-based TagLib syntax with legacy compatibility, benchmarks, and docs#15465davydotcom wants to merge 34 commits into8.0.xfrom
Conversation
…pdate - implement method-defined tag handler support and invocation context - preserve closure-style behavior across property/direct and namespaced paths - convert built-in web/GSP taglibs to method syntax - add compile-time warning for closure-defined tag fields - add coverage and benchmark for method vs closure invocation - update guides and demo taglib samples to method syntax Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
- treat only Map parameter named attrs as full tag attributes map - allow other Map-typed parameters to bind from attribute key by parameter name - add regression tests for map-valued attribute binding and reserved attrs behavior Co-Authored-By: Oz <oz-agent@warp.dev>
- use private implementation helpers to avoid recursive dispatch in typed overloads - keep Map-based handlers for validation-safe fallback behavior - add regression test ensuring private/protected methods are not exposed as tag methods - document overload pattern for typed signatures with existing validation paths Co-Authored-By: Oz <oz-agent@warp.dev>
…gnment - keep typed overloads delegating to private implementation helpers - remove unnecessary attrs.name writes since typed args are sourced from attrs - preserve behavior validated by focused FormTagLib and method-tag test suites Co-Authored-By: Oz <oz-agent@warp.dev>
- add thread-safe ClassValue cache for invokable public tag methods by name - remove per-invocation getMethods scans in hasInvokableTagMethod/invokeTagMethod - optimize TagLibrary.propertyMissing by caching method fallback closures in non-dev mode - use resolved namespace for default-namespace fallback closures Co-Authored-By: Oz <oz-agent@warp.dev>
- restore attrs-reserved binding for paginate - route namespaced method tag calls via tag output capture - add fieldValue(Map) compatibility overload - harden form fields rendering/raw handling with method dispatch Co-Authored-By: Oz <oz-agent@warp.dev>
Doc ExamplesFiles: class SimpleTagLib {
static namespace = "my"
def example() { // ← new method added
def example = { attrs -> // ← old closure NOT removed
//...
}Same issue in
|
…egistered as tag methods Make helper methods private across all affected TagLib files to prevent TagMethodInvoker.isTagMethodCandidate() from matching them as tag methods. Remove convenience overloads (e.g. textField(String,Map)) entirely where Groovy 4's multimethod restriction forbids mixing private/public methods of the same name. Changes: - ApplicationTagLib: make renderResourceLink, doCreateLink private - FormatTagLib: make messageHelper private - UrlMappingTagLib: make appendClass private - ValidationTagLib: remove fieldValue(Map) overload, make formatValue private, remove formatValue from returnObjectForTags - FormTagLib: remove 5 typed convenience overloads, make renderNoSelectionOption private - FormFieldsTagLib: make 9 protected helper methods private - TagMethodInvoker: sort methods by descending param count to prefer (Map,Closure) over (Map) signatures - Checkstyle/CodeNarc fixes: alphabetical imports, blank lines before constructors, single-quoted strings
# Conflicts: # grails-fields/grails-app/taglib/grails/plugin/formfields/FormFieldsTagLib.groovy # grails-gsp/plugin/src/main/groovy/org/grails/plugins/web/taglib/ApplicationTagLib.groovy # grails-gsp/plugin/src/main/groovy/org/grails/plugins/web/taglib/FormTagLib.groovy
|
@davydotcom I made some attempts at fixing the test pollution, but I think there is still more work required here. |
|
From my AI Results: |
|
Concerning the continued failures: Original CI failures (5 tests at maxTestParallel=3, 1 test at maxTestParallel=4) are reduced to occasional flakes on ReverseUrlMappingToDefaultActionTests.testLinkTagRendering. That remaining flake is a |
|
Update on this, from the AI: So the tag library state isn't being cleaned up correctly. |
# Conflicts: # DEVELOPMENT.md # grails-gsp/plugin/src/main/groovy/org/grails/plugins/web/taglib/ApplicationTagLib.groovy # grails-gsp/plugin/src/main/groovy/org/grails/plugins/web/taglib/FormTagLib.groovy
|
I believe I've addressed all of @jamesfredley 's comments. |
|
I decided to move the taglib cleanup to be by feature instead of by spec. The biggest reason was the flakiness of the grails-gsp test run was really due to partial clean-ups. By cleaning up after each feature, each test is isolated. |
# Conflicts: # build-logic/docs-core/build.gradle # build.gradle # grails-forge/build.gradle # grails-gradle/build.gradle
✅ All tests passed ✅Test Summary
🏷️ Commit: 38d41c8 Learn more about TestLens at testlens.app. |
codeconsole
left a comment
There was a problem hiding this comment.
Nice work!
Considerations?:
PR #15465 Review — Method-based TagLib Handlers
Substantial, well-thought refactor (+1401/-244, ~60 files). The ClassValue cache, push/pop discipline in
GroovyPage.invokeTagLibMethod, and the UrlMappingsUnitTest.mockArtefact fix are good. Highest-impact
issues below.
Blockers
B1 — TagMethodInvoker.invokeTagMethod swallows Error and rewraps it as RuntimeException. The
catch only handles RuntimeException; an Error (StackOverflowError, AssertionError) gets boxed, hiding
the true cause. Add if (target instanceof Error err) throw err; and prefer GrailsTagException
(consistent with the closure path) for non-runtime targets.
B2 — Closure-tag dispatch regresses. TagMethodInvoker.getClosureTagProperty walks the superclass
chain via getDeclaredField on every closure tag invocation, replacing what was previously a single
metaclass getProperty(tagName). Tag-heavy GSPs likely take a hit on the closure path that the benchmark
doesn't measure. Cache resolved fields in a ClassValue<Map<String, Optional<Field>>> mirroring
INVOKABLE_METHODS_BY_NAME.
B3 — TagLibArtefactTypeAstTransformation deprecation warning is unconditional. Third-party plugins
that ship closure-defined tag fields will spam warnings during user compilation, with no opt-out. Verify
whether GrailsASTUtils.warning interacts with Groovy's Werror flag — if so this can break user builds.
Gate behind a system property (grails.taglib.warnDeprecatedClosures) and/or skip when the source is from
a plugin jar, not the user's app.
Major
M1 — ValidationTagLib.formatValue silently dropped from returnObjectForTags and made private.
<g:formatValue> was a public tag; this is a breaking behavior change not called out in
upgrading80x.adoc. Either restore it as a public method tag, or document the removal.
M2 — toMethodArguments returns null (rejecting the overload) when any non-attrs/non-Closure arg
is null. Closures pass null through; typed-parameter tags now silently fall through to another
overload or throw MissingMethodException. Only reject null when the parameter is primitive; allow it
for reference types.
M3 — Method resolution depends on JVM-defined getDeclaredMethods() order for same-arity overloads.
Add a deterministic tiebreaker (e.g., parameter-type names) so behavior is identical on HotSpot vs.
Graal/J9.
M4 — is*() no-arg exclusion is heuristic-fragile. Targeted at JavaBean accessors, but a tag
legitimately written boolean isAvailable(...) works only because it has args. Consider a @Tag
annotation override for explicit opt-in.
Minor / DRY
- N1 —
FormTagLibprivate*Implhelpers (textFieldImpl,passwordFieldImpl,submitButtonImpl,
etc.) are ~30 lines of mechanical boilerplate caused by the Groovy 4 multimethod restriction. A single
private void renderField(Map attrs, String type)setting bothattrs.typeandattrs.tagNamecollapses
them. - N2 —
UrlMappingTagLib.paginate:attrs = (TypeConvertingMap) attrsre-casting the parameter to
itself is non-obvious; introduce a typed local. - N3 —
JavascriptTagLib.javascript/escapeJavascriptnot converted while everything around them is —
introduce a typed local. - N3 —
JavascriptTagLib.javascript/escapeJavascriptnot converted while everything around them is — either convert or
add a one-line comment about why. - N4 — Doc examples in
simpleTags.adocandMethodDefinedTagLibSpecreference
propertyMissing('attrs')/propertyMissing('body'). CLAUDE.md says don't expose internal mechanisms in docs — add a public
currentAttrs()/currentBody()helper on theTagLibrarytrait and document that. - N5 —
TagMethodContext.ThreadLocalis onlyremove()d when the stack empties onpop. If any dispatch path ever skips
pop, a later request on the same container thread sees staleattrs/body. Add a request-lifecycle interceptor that calls
clearAll()defensively. - N6 —
FRAMEWORK_METHOD_NAMESandNON_TAG_METHOD_NAMESoverlap conceptually; consolidate to avoid drift. - N7 — Test deletions (
RestfulReverseUrlRenderingTests-22, etc.) are not coverage losses — they're redundant after
UrlMappingCleanupInterceptorand the newmockArtefactcleanup. Good DRY.
Positives
ClassValueis the right primitive for class-keyed caches (auto-invalidates on unload).try/finallypush/pop inGroovyPage.invokeTagLibMethodis correct.- Benchmark spec is honest — prints, doesn't assertion-gate the percentage.
mockArtefactdestroys the cached singleton before re-registering — fixes a real test-isolation hazard.
Recommendation
Fix B1, B2, B3, M1, M2 before merge; the rest are post-merge improvements. The formatValue removal (M1) is the most likely to
bite real users silently

Summary
This PR introduces method-based TagLib handlers as the recommended syntax, while preserving full backward compatibility with closure-based handlers and legacy invocation paths.
Rebased onto
8.0.xfrom the original PR #15459.What's included
attrsandbody) for method handlersdef greeting(String name)binds fromname="...")Performance
The method-vs-closure benchmark added in this change set shows an approximately 7–10% improvement for method-based invocation in the covered scenarios.
TagLib syntax examples
Recommended (method-based)
Usage:
Legacy-compatible (closure field)
Validation performed
FormTagLib2TestsFormTagLib3TestsSelectTagTestsNamespacedTagLibMethodTestsTagLibMethodMissingSpecMethodDefinedTagLibSpec:grails-gsp:test:grails-test-examples-app1:integrationTest --tests functionaltests.MiscFunctionalSpecCo-Authored-By: Oz oz-agent@warp.dev